Skip to content

fix(store): a resource create never moves a live resource to another project or org - #2000

Merged
AmanGIT07 merged 1 commit into
mainfrom
fix/resource-upsert-same-project
Oct 16, 2026
Merged

AmanGIT07 merged 1 commit into
mainfrom
fix/resource-upsert-same-project

Conversation

@AmanGIT07

Copy link
Copy Markdown
Contributor

Summary

A resource create no longer moves a live resource to another project or organization. Stacked on #1985.

Changes

  • The URN upsert in ResourceRepository.Create updates the existing live row only when it has the same project_id, compared with IS NOT DISTINCT FROM so two organization resources, both without a project, still match. Otherwise no row comes back and Create returns ErrConflict, which the handlers answer with already exists.
  • The service returns before it touches SpiceDB, so a refused create leaves the existing row's tuples alone.

Technical Details

  • Two live resources from different parents share a URN only in rare cases:
    • A project named exactly like an organization id. Its resources get the URNs of that organization's resources when they share a namespace and a name. This is very unlikely, since it needs a project named after an org id and one namespace used at both levels, so the check is defensive.
    • A live resource whose project row is gone. This cannot happen today: the project delete purges its resources first, and the project_id foreign key has no delete action.
  • Re-creating a resource in the same project or the same organization still updates it in place, and the organization fill on older rows still works.

Test Plan

  • go test ./internal/store/postgres/ passes. New TestCreate cases: an organization resource re-created with the same URN updates in place; the URN of another project's live resource returns conflict and the row keeps its project; a project resource and an organization resource sharing a URN return conflict both ways.
  • Without the Where, the two conflict cases fail. With = in place of IS NOT DISTINCT FROM, the organization re-create case fails.
  • go test ./test/e2e/... passes.
  • golangci-lint run reports no issues.

SQL Safety

  • The goqu.L in the conflict update takes two goqu.I identifiers and no values.
  • ToSQL() callers capture and forward params (query, params, err := stmt.ToSQL(); db.…Context(ctx, …, query, params...)). Never query, _, err := ….
  • No ? placeholders inside single-quoted SQL literals in goqu.L.
  • No new //nolint:forbidigo or // #nosec G20x annotations.

@vercel

vercel Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
frontier Ready Ready Preview Oct 16, 2026 8:20am UTC

@coderabbitai

coderabbitai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

📝 Summary

Summary by CodeRabbit

  • Bug Fixes
    • Resource creation now prevents a project from taking over a live resource URN owned by another project or by an organization. These conflicts are reported consistently, and the existing resource’s ownership remains unchanged.
    • Recreating an organization-owned resource with the same URN updates the existing resource.

Resource creation now updates a live resource with a matching URN only when the existing and incoming project IDs match. No-row results map to resource.ErrConflict. Tests cover updates and ownership conflicts for organization-owned and project-owned resources.

Changes: Resource URN conflict handling

Layer / File(s) Summary
Ownership-aware resource creation
internal/store/postgres/resource_repository.go, internal/store/postgres/resource_repository_test.go
The upsert checks that project IDs match before updating a live resource. Create maps sql.ErrNoRows to resource.ErrConflict. Tests cover updates to an existing organization-owned resource and conflicts that preserve existing ownership.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: rohilsurana

Merge Risk: 🟡 Moderate · up to 0e1de

Creating an organization-level resource whose URN matches a live resource in another organization still moves the existing resource to the new organization. Fix the guard so it also checks the organization before merging.

Pre-merge checks: Passed 2
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coveralls

coveralls commented Oct 14, 2026 •

Copy link
Copy Markdown

Coverage Report for CI Build 38872869111

Coverage increased (+0.003%) to 54.962%

Details

  • Coverage increased (+0.003%) from the base build.
  • Patch coverage: 5 of 5 lines across 1 file are fully covered (100%).
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 41556
Covered Lines: 22840
Line Coverage: 54.96%
Coverage Strength: 19.09 hits per line

💛 - Coveralls

@rohilsurana rohilsurana left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The WHERE on the conflict update is the right tool. No row back means no move, and the service stops before SpiceDB. Approving, with one optional ask inline.

Base automatically changed from feat/resources-org-id to main October 16, 2026 08:16
…project or org

The URN upsert updates an existing live row only when it belongs to the same project, or to no project for an organization resource. Otherwise Create returns ErrConflict, so a create can no longer take over another project's or organization's row.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository: raystack/frontier/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 35c37b3e-29f0-4cc1-aca6-025eb1e55963
📥 Commits

Reviewing files that changed from the base of the PR and between 5ef510d and 0e1de17.

📒 Files selected for processing: 2
  • internal/store/postgres/resource_repository.go
  • internal/store/postgres/resource_repository_test.go

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 0 remain after this review.

@AmanGIT07
AmanGIT07 merged commit f5c763e into main Oct 16, 2026
8 checks passed
@AmanGIT07
AmanGIT07 deleted the fix/resource-upsert-same-project branch October 16, 2026 09:06

This branch was successfully deployed

1 active deployment
Preview — 0e1de17c Deployed Oct 16, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants